Skip to content

Allow caching of IPFS files in file system #6031

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

Merged
merged 11 commits into from
May 23, 2025
Merged

Allow caching of IPFS files in file system #6031

merged 11 commits into from
May 23, 2025

Conversation

lutter
Copy link
Collaborator

@lutter lutter commented May 22, 2025

This change adds a variable GRAPH_IPFS_CACHE_LOCATION, which, when set, needs to point at a directory writable by graph-node. All files that graph-node will fetch from IPFS will be cached in that directory so that subsequent requests to fetch these files, e.g., after a restart, will use those cached files rather than issuing another IPFS request.

Caching files has obvious security implications in that it is possible to modify files in the cache, and thereby change a subgraph. In other words, the cache needs to be in a trusted location with restricted access.

@lutter lutter requested a review from isum May 22, 2025 18:17
@lutter
Copy link
Collaborator Author

lutter commented May 22, 2025

@isum I reworked a good chunk of this, for a couple of reasons:

  1. I realized that we have more IPFS accesses than just the ones from IpfsResolver; I therefore moved caching lower to IpfsClient
  2. @rotarur requested that we also support Redis as a cache since that scales better than a filesystem

Sorry for troubling you again, but can you have another look at this PR?

}

fn key(path: &ContentPath) -> String {
format!("ipfs:{}", path.cid())
Copy link
Member

Choose a reason for hiding this comment

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

This ensures that we generate the correct key for requests of files within IPFS directories

Suggested change
format!("ipfs:{}", path.cid())
format!("ipfs:{path}")

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Aah .. yes! Nice catch!

Cache::Disk { store } => {
let log_err = |e: &object_store::Error| log_object_store_err(logger, e, false);

let path = Path::from(path.cid().to_string());
Copy link
Member

Choose a reason for hiding this comment

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

This is necessary to ensure that we retrieve the correct data

Suggested change
let path = Path::from(path.cid().to_string());
let path = Path::from(path.to_string());

}
Cache::Disk { store } => {
let log_err = |e: &object_store::Error| log_object_store_err(logger, e, true);
let path = Path::from(path.cid().to_string());
Copy link
Member

Choose a reason for hiding this comment

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

This is necessary to ensure that we cache the correct data

Suggested change
let path = Path::from(path.cid().to_string());
let path = Path::from(path.to_string());

@lutter lutter merged commit 12a7f50 into master May 23, 2025
6 checks passed
@lutter lutter deleted the lutter/ipfs branch May 23, 2025 18:08
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.

2 participants