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

Capture attachments in the data model to support de-serialized instance with blocks #296

Closed
vasco-santos opened this issue Apr 27, 2023 · 2 comments · Fixed by #298
Closed
Assignees

Comments

@vasco-santos
Copy link
Contributor

My only concern is that once invocation goes over the wire and is de-serialized on the other side it will loose attachments because replica.iterateIPLDBlocks will not be aware of the attachments.

I think it would be a good idea to capture these links in the data model somewhere, which leaves us with two options:

In the iterateIPLDBlocks traverse capabilities (or perhaps this.data and include all linked blocks).
Have dedicated place for sticking attached block links so we can retain behavior of iterateIPLDBlocks after they cross the wire. For example we could stick { 'ucanto/attachments': Link[] } object into fct fields and use it instead of traversing arbitrary dags.

Context: #288 (review)

@vasco-santos
Copy link
Contributor Author

@Gozala integrating this now into a client and server and we should get this iteration too.

aggregate: {
  offer: Server.provide(AggregateCapabilities.offer, ({ invocation }) => {
    // get block by CID needed
  }
}

I also wonder if we should support a convenience API to just read block by CID, thoughts?

Regarding two pointed out solutions, I would favour a non magical approach, therefore second seems more explicit. We could mutate facts adding attachments when .attach() is called. However, only invocation has them. How will we do with delegations? What do you think?

Also one question, could you give me a high level hint on how the deserialization flow is until getting a invocation object created for the server.provide function?

@Gozala
Copy link
Collaborator

Gozala commented Apr 27, 2023

Just to summarize to what we have arrived at during our call about this:

We could amend exportDAG functionality so that instead of iterating attachedLinks field it would iterate over links in the capabilities and facts fields. In fact attachedLinks can probably be turned into a lazy getter that collects all those links into a set and then exportDAG will remain as is. If we do that I would also like if attach method did not allow you adding blocks that aren't in the attachedLinks.

https://github.com/web3-storage/ucanto/blob/f72e6540a2fc323e40fe8078b9216215485e0397/packages/core/src/delegation.js#L401-L410

We do not need a separate API to dereference attached blocks because:

  1. We will have attachedLinks getter that will tell you all the links.
  2. We already have blocks field that is a map of blocks so you could de-reference from there.

https://github.com/web3-storage/ucanto/blob/f72e6540a2fc323e40fe8078b9216215485e0397/packages/interface/src/lib.ts#L210-L220

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 a pull request may close this issue.

2 participants