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: add static http gateway routing #515

Merged
merged 4 commits into from
Apr 25, 2024

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Apr 23, 2024

Adds a routing implementation that returns a static list of gateways as providers for CIDs.

It's expected that these gateways are able to fetch content on our behalf.

Depends on:

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

Adds a routing implementation that returns a static list of
gateways as providers for CIDs.

It's expected that these gateways are able to fetch content on our
behalf.
@achingbrain achingbrain requested a review from a team as a code owner April 23, 2024 06:29
}

// these values are from https://github.com/multiformats/multicodec/blob/master/table.csv
const LIBP2P_KEY_CODE = 0x72
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to use the code for transport-ipfs-gateway-http instead?

Suggested change
const LIBP2P_KEY_CODE = 0x72
const TRANSPORT_IPFS_GATEWAY_HTTP_CODE = 0x0920

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I have no strong preference here, really it just needs to be unique to allow comparison between peer identifiers.

Reading the PR that added that code the intention was for numbers to be used instead of peer protocol strings so I don't think there'll be any collision?

@2color
Copy link
Member

2color commented Apr 24, 2024

I'm curious, how do you plan on using the HTTPGatwayRouter once merged?

@achingbrain
Copy link
Member Author

how do you plan on using the HTTPGatwayRouter once merged?

It'll be configured like any other router and will let http gateway sessions fall back to a default set of gateways.

We'll also be able to switch the gateway block broker over to use the helia routing which will let it find non-preconfigured gateways but also fall back to the default ones.

Copy link
Member

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

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

looks straightforward to me but i'm not intimately familiar with PeerId class we're implementing.

i think sync multihash is fine, toCID should be unique per URL.. ship it

@SgtPooki
Copy link
Member

I'm curious, how do you plan on using the HTTPGatwayRouter once merged?

Some context for @2color from chat I had with @achingbrain when talking about issues with trustless-gateways related to testing out the changes in ipfs/helia-verified-fetch#50

Basically I’m not sure we’re ready for HTTP Gateway sessions yet.
To that end I’ve put this PR together that adds a routing implementation that just returns a static list of gateways - #515
This way when we get better HTTP results via the delegated lookup in the future we can just turn off the static list.

Essentially, we need some way to reduce the breadth of content-fetching requests (via sessions), and trustless gateways returned from delegated routing providers are not consistently reliable and frequently not available at all.

When I was looking into it, I proposed having trustless gateway sessions fall back to the provided trustless-gateways passed to blockBrokers. IIUC, this PR enables a version of that fallback in the defined "routing" language we've recently moved to in helia.

Alex to correct me here if I'm wrong or too imprecise.

@achingbrain
Copy link
Member Author

That's it, all good.

@achingbrain achingbrain merged commit 2d070b9 into main Apr 25, 2024
18 checks passed
@achingbrain achingbrain deleted the feat/add-http-gateway-routing branch April 25, 2024 09:07
@achingbrain achingbrain mentioned this pull request Apr 24, 2024
@2color
Copy link
Member

2color commented Apr 25, 2024

Nice! Cool stuff

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