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.

Sign up for GitHub

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

Jump to bottom

feat: add support for Named Records #6

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

feat: add support for Named Records #6

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b2bda1
feat: add support for Named Records
aschmahmann Jan 4, 2023
099c8da
switch to varint delimited list
aschmahmann Jan 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat: add support for Named Records
  • Loading branch information
@aschmahmann
aschmahmann authored Jan 4, 2023
commit 2b2bda1d8d25b3d7cdab365ff355427bbd1d11d7
5 changes: 5 additions & 0 deletions IPNI.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,11 @@ Specified protocols are expected to be ordered in increasing order.
* http
* the proposed `uvarint` protocol is `0x3D0000`.
* the following bytes are not yet defined.
* Named Record
* Protocol `uvarint` is <TBD> in the multicodec table
Copy link
Author

Choose a reason for hiding this comment

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

Was originally going to go for something like 0x0920 but given these are strings we can certainly add another byte and go further up the table. Once we're reasonably happy here I'll open a PR on the code table and get the number.

* the following bytes should be a dag-cbor encoded struct of:
Copy link
Author

Choose a reason for hiding this comment

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

Thoughts on if we should be saving on bytes here by making this a CBOR tuple where element 0 is the name and 1 is the Record? I took this format ro start with to mimic the GraphSync-FILv1 one.

* Name, a string
* Record, a valid dag-cbor type
Copy link
Author

Choose a reason for hiding this comment

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

Any objections here? We could make this bytes if it'll make people more comfortable but given we're already in dag-cbor land this flexibility seems fine.


Copy link
Author

Choose a reason for hiding this comment

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

Side thread: What's the reason for "Specified protocols are expected to be ordered in increasing order." and will there be problems if the same protocol is specified in the same metadata multiple times? Trying to understand if this restriction will interact with this PR in a meaningful way.

Copy link
Member

Choose a reason for hiding this comment

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

The idea with the order is that you do not have to parse the entire metadata to find if the protocol ID you are looking for is supported or not as long as a protocol with larger ID is encountered.

#### ExtendedProvider

Expand Down