Very WIP Experiment: don't require Box in protocol handler accept

[rklaehn]

Description

Add an UnboxedProtocolHandler trait and try it out in the examples.

Breaking Changes

The blanket impls for Box and Arc conflict with this, so I had to comment them out

Notes & open questions

Is this worth the effort in simplifying writing protocols?
Should ProtocolHandler always be like this, and we do the transformation to something boxable internally?

Maybe the following would be good:

• rename UnboxedProtocolHandler to just ProtocolHandler (or Protocol if we want to go this way)
• keep ProtocolHandler as BoxedProtocolHandler but hide it away a bit

That way by default people get the simple interface, but if somebody has special requirements such as

• they have an already boxed future and want to avoid boxing it again
• they don't want owned self because they want to clone not all of self but only a part
  they can still do it by implementing BoxedProtocolHandler manually. All ProtocolHandler impls get a default BoxedProtocolHandler impl.

Change checklist

• Self-review.
• Documentation updates following the style guide, if relevant.
• Tests if relevant.
• All breaking changes documented.

[github-actions]

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3020/docs/iroh/

Last updated: 2024-12-09T09:03:27Z

[github-actions]

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 1883e22

[rklaehn]

Ok, let's see how our handlers are implemented:

Gossip

impl ProtocolHandler for Gossip {
    fn accept(&self, conn: Connecting) -> BoxedFuture<Result<()>> {
        let this = self.clone();
        Box::pin(async move { this.handle_connection(conn.await?).await })
    }
}

Blobs

impl<S: crate::store::Store> ProtocolHandler for Blobs<S> {
    fn accept(&self, conn: Connecting) -> BoxedFuture<Result<()>> {
        let db = self.store().clone();
        let events = self.events().clone();
        let rt = self.rt().clone();

        Box::pin(async move {
            crate::provider::handle_connection(conn.await?, db, events, rt).await;
            Ok(())
        })
    }

    fn shutdown(&self) -> BoxedFuture<()> {
        let store = self.store().clone();
        Box::pin(async move {
            store.shutdown().await;
        })
    }
}

Docs

impl<S: iroh_blobs::store::Store> ProtocolHandler for Docs<S> {
    fn accept(&self, conn: Connecting) -> BoxedFuture<Result<()>> {
        let this = self.engine.clone();
        Box::pin(async move { this.handle_connection(conn).await })
    }

    fn shutdown(&self) -> BoxedFuture<()> {
        let this = self.engine.clone();
        Box::pin(async move {
            if let Err(err) = this.shutdown().await {
                tracing::warn!("shutdown error: {:?}", err);
            }
        })
    }
}

Gossip clones self, but arguably it should only clone inner.

So while in all examples we clone self, in all actual impls we try to clone as little as possible to move it into the future.