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

IPIP-337: Delegated Content Routing HTTP API #337

Merged
merged 29 commits into from
Feb 11, 2023
Merged

IPIP-337: Delegated Content Routing HTTP API #337

Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
1d9ec9c
feat: Delegated Routing HTTP API
guseggert Oct 18, 2022
65d178b
changes based on feedback
guseggert Oct 19, 2022
4c024dd
fix some formatting
guseggert Oct 19, 2022
0acdb01
remove unused signature field
guseggert Oct 20, 2022
f7b4437
rename to "delegated content routing" and remove IPNS
guseggert Oct 20, 2022
13d695c
use multibase-encoded payload for Provide
guseggert Oct 20, 2022
e3e744a
sign the hash of the payload
guseggert Oct 20, 2022
451b1e9
add timestamp type
guseggert Oct 20, 2022
27d23e8
adjust provider record
guseggert Oct 20, 2022
a9984a9
specify /ping not ready status code
guseggert Oct 20, 2022
fce070f
add note about non-identity-multihashed peer IDs
guseggert Oct 21, 2022
fff68c3
rework API and schema based on feedback
guseggert Nov 11, 2022
11f4ca5
formatting fix
guseggert Nov 11, 2022
39c467e
use a JSON string for payload, no reason to base-encode
guseggert Nov 11, 2022
87ff0ac
s/Multiaddrs/Addrs
guseggert Nov 11, 2022
96d55d0
properly distinguish Reframe HTTP transport from Reframe
guseggert Nov 11, 2022
4264a2d
remove dangling status code
guseggert Nov 11, 2022
0f49dcf
add -v1 suffix to filecoin-graphsync protocol name
guseggert Nov 15, 2022
7238e63
Add ID and Addrs fields to filecoin-graphsync-v1 read record
guseggert Nov 15, 2022
e823d9e
docs(http-routing): CORS and Web Browsers
lidel Nov 22, 2022
19fff93
Decouple schema from protocol in records
guseggert Dec 7, 2022
1aac44c
ipip-337: apply suggestions from review
lidel Jan 16, 2023
acc397b
chore: fix typo
lidel Jan 16, 2023
325ca1e
Reduce the scope of IPIP-337 by excluding write operations
masih Jan 24, 2023
9c47a31
Address lint issues
masih Jan 24, 2023
512bc05
Merge pull request #370 from ipfs/masih/rm_put_deleg_routing_api
lidel Jan 24, 2023
655b1f2
Rename 0000-delegated-routing-http-api.md to 0337-delegated-routing-h…
lidel Jan 24, 2023
d343189
Remove pagination and transport & transfer filters
guseggert Feb 2, 2023
573417e
ipip-337: final editorial changes
lidel Feb 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
Prev Previous commit
Next Next commit
add -v1 suffix to filecoin-graphsync protocol name
  • Loading branch information
@guseggert
guseggert committed Nov 15, 2022
commit 0f49dcffcb3811901a36c7037fa34406f7687030
6 changes: 3 additions & 3 deletions routing/DELEGATED_CONTENT_ROUTING_HTTP.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ Specifications for some transfer protocols are provided in the "Transfer Protoco

- Default limit: 100 providers
- Optional query parameters
guseggert marked this conversation as resolved.
Show resolved Hide resolved
- `transfer` only return providers who support the passed transfer protocols, expressed as a comma-separated list of transfer protocol names such as `transfer=bitswap,filecoin-graphsync`
- `transfer` only return providers who support the passed transfer protocols, expressed as a comma-separated list of transfer protocol names such as `transfer=bitswap,filecoin-graphsync-v1`
guseggert marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

@lidel lidel Jan 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Just want to flag that transfer parameter is a hardcoded list of pre-vetted strings, which is making future changes extremely difficult:

  • it will be impossible to query for providers that expose novel third-party protocols
  • every new protocol needs to be manually added to the spec here, then to libraries, and then rolled out to indexers and clients

Feels like gatekeeping system which makes it impossible to experiment with novel transfer protocols.

Last year I suggested to not invent own strings, avoid transport protocol codes from table.csv.

Since transfer is not implemented yet, I suggest removing it, or adding escape hatch for third-party protocols not specified in this document.

If we want to keep it, need to ensure we are not gatekeeping:

Option A

Ensure clients can talk the same version of bitswap by reusing strings from libp2p identify's protocols list:

$ ipfs id | jq .Protocols
[
  "/ipfs/bitswap",
  "/ipfs/bitswap/1.0.0",
  "/ipfs/bitswap/1.1.0",
  "/ipfs/bitswap/1.2.0",
  "/libp2p/fetch/0.0.1",
  ...
]

We could update spec and say that every value that starts with / should be interpreted as libp2p protocol prefix:

transfer=/ipfs/bitswap would return all peers that speak any version of bitswap.

Option B

Alternative, is to just accept number, and allow people to use codes from reserved private range

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO using the numbers from the global table feels pretty bad for experimentation as well. The existing IPNI implementation doesn't support them and they require some gatekeeping anyway. For the time being I've put up ipni/specs#6.

It seems like we could reasonably integrate this with something like the / proposal.

transfer=/ipfs/bitswap would return all peers that speak any version of bitswap.

Unfortunately, this type of query isn't supported in IPNI today and if it did it'd probably be with some specific semantics (e.g. adding a field to the "BitswapMetadata" section that's a list of protocolIDs) or a similar thing for GraphSync, unless they're going to reserve a new number every time they modify the protocol).

If we started leveraging named-record in addition to (or instead of) numbers in the global table we could just query against those. Basically, this would mean that the query could be fulfilled using custom logic per-number, or just using the named-record wrapper.

Since transfer is not implemented yet, I suggest removing it

Since the general policy is not to merge specs without implementations we should remove and then re-add later with the implementation, right?

Copy link
Member

@masih masih Jan 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

experimentation
require some gatekeeping

IPNI imposes no restriction on the protocol ID in metadata; this can be any number and treated as arbitrary bytes by the indexers.

Unfortunately, this type of query isn't supported in IPNI today

Happy to capture an issue on this if this is needed?

if it did it'd probably be with some specific semantics

It'd probably be varint prefix matching.

If we started leveraging named-record in addition to (or instead of) numbers in the global table we could just query against those

i'm sorry i am struggling to see how using names instead of numbers would make a difference in this case. Surely we can do the same with numbers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the consensus is to remove this, but the idea was that these are treated by this spec as opaque strings used for filtering, and the servers have no requirement to enforce any particular values (unless they want/need to). But happy to have this conversation another day, since we never implemented this anyway.

- `transport` for provider records with multiaddrs, only return records with multiaddrs explicitly supporting the passed transport protocols, encoded as decimal multicodec codes such as `transport=460,478` (`/quic` and `/tls/ws` respectively)
guseggert marked this conversation as resolved.
Show resolved Hide resolved
- Implements pagination according to the Pagination section

Expand Down Expand Up @@ -206,14 +206,14 @@ The `Payload` field is a string, not a proper JSON object, to prevent its conten
- If 0, the server makes no claims about the lifetime of the record


### filecoin-graphsync
### filecoin-graphsync-v1
Multicodec code: 0x0910

#### Read Provider Records

```json
{
"Protocol": "filecoin-graphsync",
"Protocol": "filecoin-graphsync-v1",
lidel marked this conversation as resolved.
Show resolved Hide resolved
"PieceCID": "<cid>",
"VerifiedDeal": true,
"FastRetrieval": true
Expand Down