Skip to content

feat: import_verifiable_stream and write_verifiable_stream in Store #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from

Conversation

matheus23
Copy link
Member

@matheus23 matheus23 commented Nov 13, 2024

Description

  • Adds a default-implemented fn Store::import_verifiable_stream
  • Adds a default-implemented fn MapEntry::write_verifiable_stream

These allow interacting with the iroh_blobs store using iroh's bao-style streams.


I'm coming across this while extracting iroh-willow into its own thing. This means merging in all work that's not yet merged into iroh yet from the willow PR/branch in iroh. This is one of these things.

IIUC, this allows us to stream the willow entry payloads directly from the WGPS protocol instead of having to simultaneously run iroh-blobs & iroh-willow.

Breaking Changes

Adding fn to traits should be non-breaking as long as they have default impls, right?

Notes & Open Questions

Unfortunately no tests with this yet, other than my pinky-promise that this code has worked so far for iroh-willow.
That said, I haven't tried bigger payloads yet.

I'll probably make use of this branch for the iroh-willow extraction and come back eventually to add proper tests.

Change checklist

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

@matheus23 matheus23 requested a review from rklaehn November 13, 2024 18:32
@matheus23 matheus23 self-assigned this Nov 13, 2024
@matheus23
Copy link
Member Author

matheus23 commented Nov 13, 2024

I still requested a review just to confirm: Is this a feature we want to support in iroh-blobs?
If so, I'll come back and add some tests to this.
If not, I won't waste time on writing tests & we'll need to think of some other way of doing this & revise the use in iroh-willow.

Copy link

github-actions bot commented Nov 13, 2024

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

Last updated: 2025-01-03T14:55:42Z

Copy link
Collaborator

@rklaehn rklaehn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it a bit weird that this is defined on MapEntry.

Wouldn't it be better to have this as a store fn that uses MapEntry? MapEntry really does everything it needs to do.

Also, it would be cool if we could make this more generic, otherwise we will have a dozen methods for special cases, like in this case streaming from offset..end of the file.

fn import_verifiable_stream<'a>(
&'a self,
hash: Hash,
total_size: u64,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does the total_size come from? Is it a verified size?

@rklaehn
Copy link
Collaborator

rklaehn commented Jan 3, 2025

write_verifiable_stream seems identical to https://docs.rs/iroh-blobs/latest/iroh_blobs/provider/fn.send_blob.html except that send_blob is more generic.

@rklaehn
Copy link
Collaborator

rklaehn commented Feb 21, 2025

Quick update on this: there are going to be a lot of changes to the store before 1.0. I am not happy with the current trait hierarchy at all. But this functionality will exist in some way in the new api.

@matheus23
Copy link
Member Author

That's perfect. I'm happy to change iroh-willow to use any eqivalent new abstractions once they're in (doesn't have to be this PR).

@rklaehn
Copy link
Collaborator

rklaehn commented Feb 21, 2025

That's perfect. I'm happy to change iroh-willow to use any eqivalent new abstractions once they're in (doesn't have to be this PR).

Basically I want to get rid of the entire trait hierarchy and replace it with a "protocol" that you use to talk to the store. There will be 2 protocol calls ImportBao and ExportBao, and these will then be wrapped in a pleasant way by an API that sits in front of the protocol.

This is the bao impex part of the protocol:

/// Export the given sizes in bao format, with the iroh block size.
///
/// The returned stream should be verified by the store.
#[derive(Debug)]
pub struct ExportBao {
    pub hash: Hash,
    pub ranges: ChunkRanges,
    pub out: mpsc::Sender<EncodedItem>,
}

#[derive(Debug)]
pub struct ImportBao {
    pub hash: Hash,
    pub size: u64,
    pub data: mpsc::Receiver<BaoContentItem>,
    pub out: oneshot::Sender<anyhow::Result<()>>,
}

And this is how it will be exposed in the "pleasant" API:

    pub async fn import_bao_quinn(
        &self,
        hash: Hash,
        ranges: ChunkRanges,
        stream: &mut quinn::RecvStream,
    ) -> anyhow::Result<()> { ... }

    pub async fn import_bao_bytes(
        &self,
        hash: Hash,
        ranges: ChunkRanges,
        data: Bytes,
    ) -> anyhow::Result<()> { ... }

@rklaehn
Copy link
Collaborator

rklaehn commented Feb 21, 2025

Closing. Will keep track of this in #61

@rklaehn rklaehn closed this Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants