Skip to content
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

feat: import_verifiable_stream and write_verifiable_stream in Store #10

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

matheus23
Copy link
Contributor

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 self-assigned this Nov 13, 2024
@matheus23
Copy link
Contributor 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

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: 2024-11-13T18:34:17Z

///
/// Data and outboard parts will be interleaved.
///
/// `offset` is the byte offset in the blob to start the stream from. It will be rounded down to
Copy link
Collaborator

@rklaehn rklaehn Nov 15, 2024

Choose a reason for hiding this comment

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

Are you sure this will round down to the next chunk group? The ranges are given in chunks, not chunk groups, so it should round down to the next chunk. So e.g. 1026 would round down to 1024.

In addition: I am sure both sides are compatible with each other, but I am not sure if they are compatible with bao without chunk groups, as specified in WGPS. Do we care?

What bao-tree will do is the following: It will round down to individual chunks, but only where needed. So e.g. if you ask for 31*1024..65*1024, it will send the chunk from 32*1024..64*1024 as a single chunk of data without any intermediate hashes. If you want to be compatible with bao without chunk groups you would have to send this as 16 individual 1024 byte chunks with intermediate hash pairs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In addition: I am sure both sides are compatible with each other, but I am not sure if they are compatible with bao without chunk groups, as specified in WGPS. Do we care?

WGPS allows payload transformations. So it's just the iroh flavor of WGPS payload transfer.

Are you sure this will round down to the next chunk group? The ranges are given in chunks, not chunk groups, so it should round down to the next chunk. So e.g. 1026 would round down to 1024.

I need to have a proper look. I've mostly shuffled around the code, I'm just the messenger :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we are OK with just being incompatible with anything else, I think it will work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants