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

Graphsync 2.0 refactors: Bind node part 2 - vouchers and voucher results #302

Open
hannahhoward opened this issue Feb 18, 2022 · 1 comment
Assignees

Comments

@hannahhoward
Copy link
Collaborator

hannahhoward commented Feb 18, 2022

Goals

This ticket builds on #301 which adds to #300. Essentially the goal is to convert voucher/voucher result types to use go-ipld-prime schema.TypedNode.

Note that if we finish this, we now introduce breaking changes to consumers of go-data-transfer

On the other hand it continues to push usable of ipld-prime farther into the stack, removing our never ending mixing of cbor-gen & go-ipld-prime

Implementation

Currently, vouchers and voucher results are something of a bizarre type. It's based off the state of our serialization/IPLD libraries at the time go-data-transfer was first written.

  1. First, every voucher is an encoding.Encodable -- which is actually a blank interface. However, in reality, for the Encode method in the encoding package to work, Encodable must be either:
  • a go-class with cbor-gen methods
  • a go-ipld-prime node
  • a class that can write to CBOR with the old go-ipld-format

What a mess!

  1. Second, every voucher is a Registerable which is just an Encodable along with a Type method that returns a TypeIdentifier (just a string)

Oh the lack of tooling we had at the time!

Honestly, I think encoding.Encodable should just become a datamodel.Node and Registerable should just be a schema.TypedNode -- you can use Type().TypeName() to get an identifier.

There's basically two approaches to this PR -- one with a smaller change set and one with a larger one:

  1. Keep the names, but make them aliases for datamodel.Node and schema.TypedNode, and then just replace the way Encode, EncodeToNode (from Graphsync 2.0 refactors: Bind node part 1 - message #301 once complete) and the methods on the Decoder interface work (including the new one from Graphsync 2.0 refactors: Bind node part 1 - message #301 ). You'll need to make sure for schema.TypedNode that the encode methods uses the .Representation() form and the decode methods uses the Prototype.Regristration().NewBuilder()

  2. Go all in and remove the type names Encodable and Registerable entirely and replace it with direct IPLD references.

Either way, you may have to:

  • modify the message constructors (depends on whether you delete encoding or keep it as an alias module)

  • handle the complications around saving vouchers into the datastore for data transfer state -- this is currently a cbor-gen data structure and I don't think we should change that for this round. the vouchers are cbg.Deferred and I think we can just encode/decode them with dagcbor.

  • updating the FakeDTType used in testing to be an schema.TypedNode :)

  • investigating what the effects this end up having in both go-fil-markets and go-legs

  • we probably need to make sure schema.TypedNode.Type().Name() matches the currently used value for all Registerable.Type() vals -- I'm not sure if there's a way to make sure a way to rename the type in a schema in case it's different from the go type name?

  • also not sure if better document that representation-level prototypes build type-level nodes ipld/go-ipld-prime#362 is an issue

@rvagg
Copy link
Member

rvagg commented May 3, 2022

FTR this has been going on in #305, with further discussion over there about approaches

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

No branches or pull requests

2 participants