feat! Remove S type parameter

[rklaehn]

Description

This removes the S type parameter from the Blobs protocol handler.

Breaking Changes

Blobs no longer has the S type parameter. As a consequence, Blobs can no longer have the store available. So any code that needs direct access to the store can no longer get that via the protocol handler.

• Blobs<S> -> Blobs
• Blobs::store is removed (you need to keep a separate reference to the store)
• Blobs::handle_rpc_request removed (you need to create a separate RpcHandler)
• Blobs::rt renamed to Blobs::local_pool_handle

Notes & open questions

Rpc requires knowing the type of the store. But Blobs does not have that info. So the way it works now is that you can build a rpc handler using the blobs builder, and then get the blobs protocol handler from that.

Not super nice, but I can't see another way to do this since once the type info is gone there is no way to get it back.

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-blobs/pr/24/docs/iroh_blobs/

Last updated: 2024-12-02T13:26:30Z

[dignifiedquire]

why do you need to clone, shouldn't this already be owned?

[rklaehn]

self is Arc (has to be to comply with the ProtocolHandler trait, but I need an Arc, not an &Arc.

This is what I mean when I complain about the Arcs being contagious.

[matheus23]

Have we considered an enum instead of a trait/abstraction here?

I.e.

enum BlobStore {
    InMemory(iroh_blobs::store::mem::Store),
    FileSystem(iroh_blobs::store::fs::Store),
}

impl iroh_blobs::store::Store for BlobStore {
    // ...
}

The argument against this would be a custom blob store implementation. So far I haven't seen anyone do that? Entirely possible that that's because I haven't looked enough yet. Please tell me if that's the case 😬

There's still a bunch more things we could do before going this route, e.g. a feature-flagged (and #[non_exhaustive]) enum in iroh_blobs should there be a need for a third option.

[rklaehn]

The argument against this would be a custom blob store implementation. So far I haven't seen anyone do that? Entirely possible that that's because I haven't looked enough yet. Please tell me if that's the case 😬

I did it to do a blob store backed by s3 buckets, see https://github.com/n0-computer/iroh-experiments/tree/main/iroh-s3-bao-store . This was due to customer demand or at least as a demo for a prospective customer.

Also I really want to keep the traits so I can experiment with something like fjall without having to rewrite all of blobs.

[rklaehn]

@dignifiedquire @matheus23 I think main motivation for getting rid of the S type parameter for Blobs was to allow better interaction with get_protocol (having to provide S there was weird). So now that get_protocol is no more, I think it is no longer super important.

So should we just close this? I don't think the complexity of having a two stage builder and internal dynamic dispatch is warranted anymore...

[rklaehn]

Getting rid of S is no longer that important now that we removed get_protocol...